Introduce supervisor.procHandle interface #5137
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Tom Wieczorek <[email protected]>
Signed-off-by: Tom Wieczorek <[email protected]>
Add the cmdline and environ functions to it. Signed-off-by: Tom Wieczorek <[email protected]>
This encapsulates the kill syscall. Signed-off-by: Tom Wieczorek <[email protected]>
pkg/supervisor/supervisor_unix.go
Outdated
Comment on lines
39
to
41
| // killPid tries to gracefully terminate a PID. If it's still running after | ||
| // s.TimeoutStop the process will be terminated forcibly. | ||
| func (s *Supervisor) killPid(ph procHandle) error { |
There was a problem hiding this comment.
I think it would make more sense to avoid the term PID entirely in this function because the interface is an abstraction that doesn't include the PID itself, even though it's there underneath.
Suggested change
| // killPid tries to gracefully terminate a PID. If it's still running after | |
| // s.TimeoutStop the process will be terminated forcibly. | |
| func (s *Supervisor) killPid(ph procHandle) error { | |
| // killProcess tries to gracefully terminate a Process. If it's still running after | |
| // s.TimeoutStop the process will be terminated forcibly. | |
| func (s *Supervisor) killProcess(ph procHandle) error { |
This encapsulates all of the OS-specific code for interacting with processes identified by PID files. Signed-off-by: Tom Wieczorek <[email protected]>
This will be the new factory function for OS-specific process interaction. Signed-off-by: Tom Wieczorek <[email protected]>
Move the now OS-agnostic Supervisor methods maybeKillPidFile, shouldKillProcess and killPid from the UNIX-specific file into the general one. Signed-off-by: Tom Wieczorek <[email protected]>
The supervisor code is now completely OS independent. It uses the procHandle interface to abstract away any OS specifics. Rename the old OS-specific supervisor files to prochandle, since that's what they contain now. Signed-off-by: Tom Wieczorek <[email protected]>
twz123
force-pushed
the
supervisor-prochandle
branch
from
1186969
to
ceaa0f9
Compare
juanluisvaladas
approved these changes
Oct 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This encapsulates all OS-specific code for interacting with processes identified by PID files. This allows OS agnostic code parts to be shared across OSes, and it opens a way to more easily add Windows support in the future.
The OS specific features are:
The code has been refactored to first introduce freestanding functions for command line and environment inspection, then add a new type to which these functions have been moved and which is used by the OS agnostic methods. Finally, the methods have been moved out of platform-specific files, and the platform-specific files have been renamed to reflect that they belong to the new
procHandleabstraction.Type of change
How Has This Been Tested?
Checklist: